fix: make clients shared across all users while keeping work entries per-user#482
fix: make clients shared across all users while keeping work entries per-user#482devin-ai-integration[bot] wants to merge 1 commit intomainfrom
Conversation
…per-user - Remove user_email filtering from all client queries (GET, PUT, DELETE) - Keep user_email in INSERT as audit field (created_by) - Remove bulk DELETE / endpoint for clients (dangerous for shared resources) - Update client verification in workEntries to not check user_email - Update client lookups in reports to not check user_email - Work entry queries remain scoped to individual users - Remove idx_clients_user_email index (no longer used for filtering) - Update all related tests
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| } | ||
| }); | ||
|
|
||
| // Delete all clients for authenticated user |
There was a problem hiding this comment.
🔴 Frontend "Clear All" button calls removed backend endpoint, causing 404
The PR removed the DELETE /api/clients (delete-all) route handler from backend/src/routes/clients.js:189-208 (old lines), but the frontend still references it. frontend/src/api/client.ts:84-87 defines deleteAllClients() which calls DELETE /api/clients, and frontend/src/pages/ClientsPage.tsx:83-92 wires it into a deleteAllMutation rendered as a "Clear All" button at ClientsPage.tsx:174-184. When a user clicks this button, the request will get a 404 since no route handler matches DELETE /api/clients anymore (the remaining DELETE /:id route won't match an empty id segment).
Prompt for agents
The DELETE / route for clients was removed from backend/src/routes/clients.js, but the frontend still uses it. Either:
1. Remove the frontend references: delete the deleteAllClients() method in frontend/src/api/client.ts:84-87, remove deleteAllMutation in frontend/src/pages/ClientsPage.tsx:83-92, and remove the Clear All button in ClientsPage.tsx:174-184.
2. Or re-add a DELETE / route to the backend if clearing all clients is still a desired feature (though in a shared-clients model this would be very destructive and should probably be removed).
Option 1 is recommended since deleting all shared clients would be a dangerous operation in the new shared model.
Was this helpful? React with 👍 or 👎 to provide feedback.
| db.get( | ||
| 'SELECT id FROM clients WHERE id = ? AND user_email = ?', | ||
| [clientId, req.userEmail], | ||
| 'SELECT id FROM clients WHERE id = ?', | ||
| [clientId], |
There was a problem hiding this comment.
🔴 Removing user_email check on client delete allows any user to cascade-delete other users' work entries
The authorization check (AND user_email = ?) was removed from the client delete endpoint at backend/src/routes/clients.js:201-202. Since the work_entries table has FOREIGN KEY (client_id) REFERENCES clients (id) ON DELETE CASCADE (backend/src/database/init.js:64), any authenticated user can now delete any client, which cascades to destroy ALL work entries from ALL users referencing that client. Previously, only the client's owner could delete it, so the cascade was safe. Now User A can delete a client created by User B, silently destroying User B's (and all other users') work entries for that client. This contradicts the per-user isolation still enforced on work entry routes (workEntries.js:62, workEntries.js:161, workEntries.js:272).
Prompt for agents
The client delete endpoint no longer checks user_email, but due to ON DELETE CASCADE on the work_entries foreign key, deleting a shared client destroys all users' work entries for that client. This is a cross-user data loss vulnerability.
Possible fixes:
1. Before deleting a client, check that no other users have work entries referencing it (or only allow deletion if the only work entries belong to the requesting user).
2. Remove CASCADE from the foreign key and instead handle work entry cleanup explicitly — e.g., only delete the requesting user's work entries and reassign or orphan others.
3. Restrict client deletion to the original creator (keep user_email check on DELETE but remove it from read operations).
4. Add a soft-delete mechanism instead of hard-deleting clients.
The fix should be applied in backend/src/routes/clients.js in the DELETE /:id handler, and potentially also in the database schema at backend/src/database/init.js.
Was this helpful? React with 👍 or 👎 to provide feedback.
Summary
Clients are no longer scoped per-user. All authenticated users can view, edit, and delete any client. Work entries remain scoped to individual users.
Key changes:
clients.js: Removeduser_emailfiltering from all GET/PUT/DELETE queries. Removed the bulkDELETE /endpoint (dangerous for shared resources).user_emailis still written on INSERT as an audit trail (created_by).workEntries.js: Client verification queries no longer checkuser_email. Error messages updated from "Client not found or does not belong to user" → "Client not found". Allwork_entriesqueries that filter byuser_emailare unchanged.reports.js: Client lookup queries no longer checkuser_email. Work entry queries within reports still filter byuser_email.init.js(both locations): Removedidx_clients_user_emailindex since it's no longer used for filtering. Theuser_emailcolumn itself is retained in the schema.Review & Testing Checklist for Human
DELETE /api/clients(bulk delete): This endpoint was removed entirely. Any frontend code relying on it will get a 404 or unexpected behavior.work_entriesqueries lost theiruser_emailfilter — only client-related queries should have changed.Notes
user_emailcolumn and foreign key constraint remain in theclientstable schema — it now serves purely as an audit field recording who created the client.idx_clients_user_emailindex harmlessly; it just won't be created for new databases.Link to Devin session: https://partner-workshops.devinenterprise.com/sessions/3913cfab822046b8934783bf9a187f69